Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support AT_SYSINFO and AT_SYSINFO_EHDR on all architectures #13

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pattop
Copy link

@pattop pattop commented Nov 21, 2023

Not sure why AT_SYSINFO and AT_SYSINFO_EHDR were limited to x86 and x86/x86_64 respectively.

Linux sets AT_SYSINFO on ia64 and x86 and AT_SYSINFO_EHDR on all architectures.

Removing the the architecture guard on AT_SYSINFO is harmless as the identifier (32) is not reused on other architectures, so the code will work correctly without an architecture guard.

Not sure why AT_SYSINFO and AT_SYSINFO_EHDR were limited to x86 and
x86/x86_64 respectively.

Linux sets AT_SYSINFO on ia64 and x86 and AT_SYSINFO_EHDR on all architectures.

Removing the the architecture guard on AT_SYSINFO is harmless as the
identifier (32) is not reused on other architectures, so the code will
work correctly without an architecture guard.

Signed-off-by: Patrick Oppenlander <[email protected]>
@pattop pattop requested a review from a team as a code owner November 21, 2023 04:24
@pattop pattop requested review from dpal and removed request for a team November 21, 2023 04:24
@rjzak
Copy link
Member

rjzak commented Nov 21, 2023

Would this inhibit using the crate on Linux for other platforms, like arm?

@npmccallum
Copy link
Member

@pattop @rjzak See the documentation here: https://man7.org/linux/man-pages/man3/getauxval.3.html

@pattop
Copy link
Author

pattop commented Nov 21, 2023

Would this inhibit using the crate on Linux for other platforms, like arm?

No, this fixes the crate on other platforms. It's currently broken on everything other than x86 and x86_64.

@pattop
Copy link
Author

pattop commented Nov 21, 2023

@pattop @rjzak See the documentation here: https://man7.org/linux/man-pages/man3/getauxval.3.html

That's a very good point. I raised this to fix https://github.com/enarx/vdso on arm64. But in hindsight the vdso crate really should be using the libc function.

This is broken in other ways too. You can't rely on the environ pointer as a way to access auxv as setting any environment variable in the process will change environ to point somewhere else leading to undefined behaviour in from_environ() as it walks off the end of the array.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants